fix: use server-issued session ID for HTTP backend tools/list#1559
fix: use server-issued session ID for HTTP backend tools/list#1559
Conversation
When registerToolsFromBackend called SendRequestWithServerID, it
injected a fabricated 'gateway-init-{serverID}' session ID into the
context. In sendHTTPRequest, the context session ID has higher priority
than the conn.httpSessionID captured during initializeHTTPSession(),
so the real server-issued session ID was silently overridden.
Backends like Datadog (Streamable HTTP / 2025-03-26) issue a specific
Mcp-Session-Id during initialize and reject subsequent requests that
use any other session ID with HTTP 400. This caused tools/list to fail
and the backend to register zero tools, even though the health check
reported the server as 'running'.
Fix: remove the fabricated context session ID from
registerToolsFromBackend. sendHTTPRequest now correctly falls back to
conn.httpSessionID (the real session ID from initialize) for all
startup-time tool registration requests.
Fixes: github/gh-aw#18712
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes HTTP backend startup tool discovery by ensuring tools/list uses the server-issued Mcp-Session-Id captured during initialize, rather than overriding it with a gateway-fabricated session ID. This aligns the gateway behavior with strict Streamable HTTP MCP backends (e.g., Datadog) that require the exact session ID they issue.
Changes:
- Remove the fabricated
"gateway-init-{serverID}"context session ID fromregisterToolsFromBackendsosendHTTPRequestfalls back to the storedconn.httpSessionID. - Rewrite
TestHTTPBackendInitializationas a regression test assertingtools/listuses the server-issued session ID and fails with HTTP 400 otherwise.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/server/unified.go | Stops injecting a fake session ID into the context for startup-time tools/list, allowing the stored server-issued ID to be used. |
| internal/server/unified_http_backend_test.go | Updates initialization test to model strict backends that issue/require a specific Mcp-Session-Id, validating the regression scenario. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| }, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
The mock server handler doesn’t send any response for unexpected JSON-RPC methods (falls through the switch), which results in a 200 with an empty body. That can make failures hard to diagnose if startup behavior changes (e.g., additional methods during init). Consider adding a default case that returns a JSON-RPC error / HTTP 400 (or calls t.Errorf) so unexpected methods fail fast.
| }) | |
| }) | |
| default: | |
| // Fail fast on unexpected JSON-RPC methods so tests are easier to diagnose. | |
| t.Errorf("unexpected JSON-RPC method %q", req.Method) | |
| w.WriteHeader(http.StatusBadRequest) | |
| w.Header().Set("Content-Type", "application/json") | |
| _ = json.NewEncoder(w).Encode(map[string]interface{}{ | |
| "jsonrpc": "2.0", | |
| "error": map[string]interface{}{"code": -32601, "message": "Unexpected method"}, | |
| "id": 1, | |
| }) |
| const serverSessionID = "server-issued-session-42" | ||
| var toolsListSessionID string | ||
|
|
There was a problem hiding this comment.
toolsListSessionID is written from the httptest server handler goroutine and read from the main test goroutine without synchronization. This will trigger the Go race detector and can be flaky. Consider capturing the session ID via a buffered channel, sync.Mutex, or atomic.Value and reading it after NewUnified completes.
Problem
HTTP backends that implement the Streamable HTTP MCP transport (spec 2025-03-26) issue a specific
Mcp-Session-Idduringinitializeand require that exact session ID on all subsequent requests. Whentools/listarrived with a different session ID, they returned HTTP 400, causing the gateway to register zero tools for the backend — even though the health check reported the server asrunning.Root cause:
registerToolsFromBackendinjected a fabricated"gateway-init-{serverID}"session ID into the request context. InsendHTTPRequest, the context session ID has higher priority thanconn.httpSessionID(the real session ID captured frominitializeHTTPSession()), so the server-issued ID was silently overridden on every startup-timetools/listcall.Reported in: github/gh-aw#18712 (Datadog MCP server affected)
Fix
Remove the fabricated context session ID from
registerToolsFromBackendand passcontext.Background()directly.sendHTTPRequestalready falls back toconn.httpSessionIDwhen no context session ID is present, so it now correctly uses the server-issued session ID.Tests
TestHTTPBackendInitializationis rewritten as a regression test for this exact scenario:Mcp-Session-Idduringinitializetools/listwith HTTP 400 if the session ID doesn't matchAll existing tests continue to pass.